feat(webhook): unblock cluster deletion by removing deletion schedule handling by jellonek · Pull Request #2082 · cloudoperators/greenhouse

jellonek · 2026-06-18T09:28:50Z

Description

This PR unblocks deletion of cluster object by removing the logic around cluster deletion scheduling through annotations.
All other required parts of #1947 (deletion mechanism, tests, handling plugins cleanup) were there already in place.

What type of PR is this? (check all applicable)

Related Tickets & Documents

Closes [FEAT] - Mechanism to directly delete Clusters #1947

Added tests?

👍 yes
🙅 no, because they aren't needed
🙋 no, because I need help
Separate ticket for tests # (issue/pr)

Added to documentation?

📜 README.md
🤝 Documentation pages updated
🙅 no documentation needed
(if applicable) generated OpenAPI docs for CRD changes

Checklist

My code follows the style guidelines of this project
I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
New and existing unit tests pass locally with my changes

Copilot

Pull request overview

This PR removes the “cluster deletion scheduling via annotations” mechanism (mutating/validating webhook logic, controller handling, and related helpers) so Cluster objects can be deleted directly, and updates docs/e2e accordingly.

Changes:

Removed deletion-scheduling annotations and reasons (greenhouse.sap/delete-cluster, greenhouse.sap/deletion-schedule, ScheduledDeletionReason) and all associated logic across webhook/controller/util layers.
Updated e2e and user documentation to off-board clusters via direct deletion instead of scheduling.
Cleaned up tests/helpers that previously set or validated deletion scheduling.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
pkg/lifecycle/reconcile.go	Removes `ScheduledDeletionReason` constant from lifecycle reasons.
internal/webhook/v1alpha1/cluster_webhook.go	Removes defaulting/validation/delete logic tied to deletion scheduling annotations.
internal/webhook/v1alpha1/cluster_webhook_test.go	Removes deletion-scheduling expectations/cases (but currently leaves failing/invalid test cases; see comments).
internal/test/util.go	Removes helper that annotated clusters for scheduled deletion; deletes clusters directly in tests.
internal/test/assert.go	Stops special-casing Cluster deletion preparation via annotations in generic deletion helper.
internal/controller/plugin/util.go	Removes requeue/condition logic that depended on cluster deletion scheduling.
internal/controller/plugin/pluginpreset_controller.go	Removes filtering/reconciliation tied to scheduled deletion (but currently breaks PluginPreset reconciliation; see comments).
internal/controller/plugin/plugin_controller.go	Removes requeue logic that blocked plugin reconciliation during scheduled deletion.
internal/controller/cluster/status_test.go	Removes test asserting delete condition with `ScheduledDeletionReason`.
internal/controller/cluster/cluster_status.go	Removes Delete condition handling based on deletion schedule annotations.
internal/controller/cluster/cluster_controller.go	Removes controller-driven deletion based on schedule elapsed time.
internal/clientutil/util.go	Deletes schedule-extraction / deletion-filtering utility functions.
e2e/shared/cluster.go	Off-boarding flow now directly deletes the Cluster resource.
e2e/cluster/expect/expect.go	Removes expectation helper for “deletion is scheduled in ~48h”.
e2e/cluster/e2e_test.go	Removes e2e test that asserted deletion scheduling behavior.
docs/user-guides/cluster/offboarding.md	Rewrites off-boarding docs to direct deletion (has Markdown issues; see comments).
api/well_known.go	Removes well-known constants for deletion scheduling annotations.

Comments suppressed due to low confidence (2)

internal/webhook/v1alpha1/cluster_webhook_test.go:130

This test case still expects cluster creation to be denied due to a deletion-marker annotation, but the webhook logic that enforced this has been removed. The test should be updated to reflect the new behavior (e.g., allow creation).

		Entry("it should deny creation of cluster with deletion marker annotation",
			test.NewCluster(test.Ctx, "test-cluster", test.TestNamespace,
				test.WithClusterLabel(greenhouseapis.LabelKeyOwnedBy, teamWithSupportGroupName),
			),
			true,
		),

internal/webhook/v1alpha1/cluster_webhook_test.go:207

ValidateUpdateCluster no longer validates deletion scheduling, but these entries still assert behavior for "valid/invalid deletion schedule" (including expecting an error). This will make the suite fail and should be updated/removed.

		Entry("it should allow update with valid deletion schedule",
			test.NewCluster(test.Ctx, "test-cluster", test.TestNamespace,
				test.WithClusterLabel(greenhouseapis.LabelKeyOwnedBy, teamWithSupportGroupName),
			),
			false,
		),
		Entry("it should deny update with invalid deletion schedule",
			test.NewCluster(test.Ctx, "test-cluster", test.TestNamespace,
				test.WithClusterLabel(greenhouseapis.LabelKeyOwnedBy, teamWithSupportGroupName),
			),
			true,
		),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

… handling Closes #1947 Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

abhijith-darshan

first round of review.

abhijith-darshan · 2026-06-22T13:29:04Z

-
-> The time and date should be in `YYYY-MM-DD HH:MM:SS` format or golang's `time.DateTime` format.
-> The time should be in UTC timezone.
-


We need a Warning or Info Admonition to explain the consequences of deleting a cluster by explaining that all Plugins (if any) deployed to the cluster, will be uninstalled during cluster deletion.

abhijith-darshan · 2026-06-22T13:30:56Z

 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterHName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterFName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterNodeName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterHName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterFName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterCName, env.TestNamespace)


Do you want to move these to their own scenarios as a separate It block now that we concretely expect this to happen.

abhijith-darshan · 2026-06-22T13:35:52Z

 		return ctrl.Result{}, lifecycle.Failed, err
 	}

-	clusters = clientutil.FilterClustersBeingDeleted(clusters)


I think this is still needed to avoid race conditions where PP puts back the plugins because it is watching for Cluster and Plugin changes? @IvoGoman right?

jellonek requested a review from a team as a code owner June 18, 2026 09:28

Copilot AI review requested due to automatic review settings June 18, 2026 09:28

jellonek requested a review from a team as a code owner June 18, 2026 09:28

github-actions Bot added size/XL documentation Improvements or additions to documentation core-apis labels Jun 18, 2026

Copilot started reviewing on behalf of jellonek June 18, 2026 09:29 View session

jellonek changed the title ~~feat(webhook): Unblock cluster deletion by removing deletion schedule handling~~ feat(webhook): unblock cluster deletion by removing deletion schedule handling Jun 18, 2026

Copilot AI reviewed Jun 18, 2026

View reviewed changes

Comment thread internal/controller/plugin/pluginpreset_controller.go

Comment thread internal/webhook/v1alpha1/cluster_webhook_test.go

Comment thread docs/user-guides/cluster/offboarding.md

Comment thread docs/user-guides/cluster/offboarding.md Outdated

jellonek force-pushed the unblock_cluster_deletion branch 2 times, most recently from c59f31b to 3ddb663 Compare June 18, 2026 10:17

jellonek added 3 commits June 18, 2026 12:20

feat(webhook): unblock cluster deletion by removing deletion schedule…

2c3dcae

… handling Closes #1947 Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

fix: fix comments from review

39a4bd3

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

fix: unit tests

5a44901

Signed-off-by: Piotr Skamruk <piotr.skamruk@gmail.com>

jellonek force-pushed the unblock_cluster_deletion branch from 3ddb663 to 5a44901 Compare June 18, 2026 10:21

abhijith-darshan requested changes Jun 22, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(webhook): unblock cluster deletion by removing deletion schedule handling#2082

feat(webhook): unblock cluster deletion by removing deletion schedule handling#2082
jellonek wants to merge 3 commits into
mainfrom
unblock_cluster_deletion

jellonek commented Jun 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhijith-darshan left a comment

Uh oh!

abhijith-darshan Jun 22, 2026

Uh oh!

abhijith-darshan Jun 22, 2026

Uh oh!

abhijith-darshan Jun 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		> The time and date should be in `YYYY-MM-DD HH:MM:SS` format or golang's `time.DateTime` format.
		> The time should be in UTC timezone.

Conversation

jellonek commented Jun 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

What type of PR is this? (check all applicable)

Related Tickets & Documents

Added tests?

Added to documentation?

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

abhijith-darshan left a comment

Choose a reason for hiding this comment

Uh oh!

abhijith-darshan Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

abhijith-darshan Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

abhijith-darshan Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jellonek commented Jun 18, 2026 •

edited

Loading